refactor: move encoding logic to aind-video-utils#35
Merged
Conversation
galenlynch
force-pushed
the
refactor/remove-encoding-logic
branch
from
409a16d to
691bdf5
Compare
galenlynch
force-pushed
the
refactor/remove-encoding-logic
branch
from
8638d09 to
1fa4f96
Compare
Member
Author
|
Coverage lowered because the PR eliminated lines of code, but did not change the test coverage. |
There was a problem hiding this comment.
Pull request overview
Refactors this ETL package to source ffmpeg encoding presets from aind-video-utils rather than maintaining hardcoded preset strings locally.
Changes:
- Replaced in-repo ffmpeg preset enums/strings with a mapping to
aind-video-utilsEncodingProfiles and converted profiles into CLI arg strings. - Added
aind-video-utilsas a runtime dependency. - Lowered the coverage enforcement threshold in
pyproject.toml.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/aind_behavior_video_transformation/transform_videos.py |
Switches preset resolution to aind-video-utils encoding profiles and generates ffmpeg args from those profiles. |
pyproject.toml |
Adds the new dependency and adjusts coverage gate settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
galenlynch
added a commit
that referenced
this pull request
May 20, 2026
Address Copilot review on #35: - Key _COMPRESSION_PROFILES by the enum directly instead of stringly-typed member names so a future rename of CompressionEnum can't silently break preset lookup, and the DEFAULT alias is colocated with the rest of the map. - Coerce None user_ffmpeg_{input,output}_options to "" when returning from the USER_DEFINED branch so determine_ffmpeg_arg_set actually honors its declared Tuple[str, str] return type. Empty strings are already treated as "no extra args" by convert_video, so behavior is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jtyoung84
approved these changes
Jun 26, 2026
jtyoung84
left a comment
jtyoung84
left a comment
Contributor
There was a problem hiding this comment.
It looks like this is removing classes, which is a breaking change and not just a refactor.
Member
Author
|
I’ll change the commit messages to reflect that |
BREAKING CHANGE: FfmpegInputArgs, FfmpegOutputArgs removed ci: lower coverage threshold to 90 Removing the FfmpegInputArgs/FfmpegOutputArgs/FfmpegArgSet enums shrank the statement denominator in transform_videos.py, dropping total coverage from 92% to 91% even though no test behavior changed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> refactor: key compression profile dict by CompressionEnum Address Copilot review on #35: - Key _COMPRESSION_PROFILES by the enum directly instead of stringly-typed member names so a future rename of CompressionEnum can't silently break preset lookup, and the DEFAULT alias is colocated with the rest of the map. - Coerce None user_ffmpeg_{input,output}_options to "" when returning from the USER_DEFINED branch so determine_ffmpeg_arg_set actually honors its declared Tuple[str, str] return type. Empty strings are already treated as "no extra args" by convert_video, so behavior is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> chore: bump aind-video-utils dep
galenlynch
force-pushed
the
refactor/remove-encoding-logic
branch
from
ee23a48 to
0d0c9fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have moved the aind-file-standards encoding presets into aind-video-utils, instead of having it baked into this ETL package. I think we should use aind-video-utils as the source of truth for encoding presets moving forward.